Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4474 +/- ##
==========================================
+ Coverage 72.42% 72.64% +0.22%
==========================================
Files 975 975
Lines 46617 46688 +71
Branches 10069 10082 +13
==========================================
+ Hits 33760 33912 +152
+ Misses 11857 11786 -71
+ Partials 1000 990 -10 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
a0f3012 to
1815695
Compare
|
⏩ The changelog entry check has been skipped since the "no changelog" label is present. |
|
/devin review |
|
Starting Devin Review. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| .then(async () => { | ||
| try { | ||
| await access(path.join(outputDirectory, 'session.json')); | ||
| } catch { | ||
| scheduleRecordingRetry(session, udid); | ||
| } |
There was a problem hiding this comment.
🟡 Late manifests trigger duplicate recordings
If session.json appears after a successful client exit, completionPromise schedules a retry despite the later manifest wait. The poller can start another recording for the same server token, producing duplicate session recordings.
Learn more
A recorder's process exit and the manifest's appearance are treated as separate events by waitForRecordingManifestAsync, which allows 30 seconds for the manifest. This immediate post-exit check instead counts a temporarily absent manifest as a failed attempt. pollIosSimulatorRecordingsAsync is allowed to launch another recorder after the 25-second retry delay, even if the original manifest has since appeared.
Example: The client exits at 10:00:00, and its manifest appears at 10:00:03. The access check at 10:00:00 schedules a retry at 10:00:25. If serve-sim is still registered at that point, a second recording starts despite the completed first recording.
Recommended fix: Apply the same bounded manifest grace period before scheduling a retry, or recheck the manifest immediately before restarting a recorder for an unchanged server token.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const token = readyServers.get(device.udid); | ||
| const packageSpec = serveSimPackageSpecs.get(device.udid); | ||
| if (!token || !packageSpec) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
🔍 Recording coverage depends on preview startup
A booted simulator now records only while a token-bearing serve-sim preview has registered its package. Check whether workflows need footage before preview startup or without a preview.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Confirmed: recording starts when a token-bearing serve-sim preview is ready, so this PR does not capture footage before that startup. I added that boundary explicitly to the PR description. It follows the planned single native capture path; restoring earlier footage would need a different owner for capture before serve-sim starts.
— Codex
1815695 to
cff9ab3
Compare
|
/devin review |
|
Starting Devin Review. |
1 participant
Why
The session runner used record-sim to capture the framebuffer separately from serve-sim. The new serve-sim stack owns one native capture and a dedicated hardware recording encoder, so build-tools can use its recording client and keep the existing upload manifest.
Depends on expo/serve-sim#208 and its lower stack layers being released first.
How
Start
serve-sim record-videofrom the selected serve-sim package after its token-bearing preview is ready, wait for its start marker, and stop with SIGINT. Recording begins with that preview; footage before serve-sim startup is outside this recording. If serve-sim stops first, it finalizes the file during shutdown while the client waits for the manifest. Give the serve-sim process group up to 90 seconds after SIGTERM before SIGKILL, even when its package-manager wrapper exits early. Remove record-sim packaging; the upload step is unchanged.Test Plan
corepack yarn jest-unit --runInBand src/steps/utils/__tests__/IosSimulatorRecordingUtils.test.ts src/steps/utils/__tests__/remoteDeviceRunSession.test.ts: 74 passed.corepack yarn typecheckin build-tools, rootcorepack yarn lint, and rootcorepack yarn fmt:check: passed.— Codex